-
Notifications
You must be signed in to change notification settings - Fork 3
feat: Update all colors to use the DSDL #539
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✅ Deploy Preview for cal-itp-website ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
jgravois
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
personally i think the color discrepancies you called out are improvements.
beyond the fact that this PR deletes more code than it adds, i'd say the styles that remain are more legible/maintainable. kudos!
|
|
||
| .black-on-white .nav-pills .nav-link:hover { | ||
| border-color: rgba(33, 33, 33, 0.8); | ||
| border-color: rgba(38, 38, 38, 0.8); /* gray 80 at 80% opacity */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this might be a little too clever, but TIL it's possible derive RGB on the fly.
| border-color: rgba(38, 38, 38, 0.8); /* gray 80 at 80% opacity */ | |
| border-color: rgb(from var(--dsdl-gray-80) r g b / 0.8); |
browser support is currently slightly > the opacity (~85%) ![]()
https://caniuse.com/mdn-css_types_color_rgb_relative_syntax
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh wow, that is cool! It does feel maybe slightly too bleeding-edge right now (just landed in Safari in fall 2024), but this could provide a nice path forward in the near future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this could provide a nice path forward in the near future.
given our target audience, i agree.
| --calitp-red-4: rgb(192, 63, 77); /* #c03f4d */ | ||
| --calitp-red-5: rgb(160, 46, 59); /* #a02e3b */ | ||
| --calitp-yellow-4: rgb(253, 183, 20); /* #FDB714 */ | ||
| --calitp-font-weight-bold: 700; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔪 very nice. 🔪
ℹ️ PR is to base branch
feature/dsdlℹ️Closes #530
There's a lot here, but it's largely search and replace work.
There are some subtle differences that can be observed, mainly in the background colors (main nav hover BG and homepage Initiatives tabs, for example). I followed the mapping provided in the DSDL Figma file to convert those.
This also starts to transition some responsibility for site-wide styles from Bootstrap to the DSDL, as discussed a bit during our workshop today. As a result, many of the existing Bootstrap variable overrides were able to be discarded. (And specifically, I didn't uncover any need – at least on this site – to generate
r, g, bstrings to maintain those-rgbvariables.)Minor additions to the DSDL itself that I found helpful to add here:
--dsdl-body-colorvariable, to replace the use of--bs-body-colorand so that other potential DSDL consuming sites could set their own default body color..text-brand-blueutility class for the couple of occasions where we are setting regular text to that color.Note: Due to importing the whole DSDL now at the top of
main.css, some typography changes are now visible, but the work to finish those will come in a separate PR.